Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): add espresso-reader #150

Draft
wants to merge 8 commits into
base: feature/cli-run-v2
Choose a base branch
from

Conversation

endersonmaia
Copy link
Contributor

@endersonmaia endersonmaia commented Feb 3, 2025

You can test this with:

pnpm build --filter @cartesi/cli
alias cartesi-dev=$PWD/apps/cli/bin/run.js
cartesi-dev create --branch=prerelease/sdk-12 --template=go my-go-dapp-v2 
cd my-go-dapp-v2
cartesi-dev build
cartesi-dev run --enable-espresso

@endersonmaia endersonmaia self-assigned this Feb 3, 2025
Copy link

changeset-bot bot commented Feb 3, 2025

⚠️ No Changeset found

Latest commit: dc4d99a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@endersonmaia endersonmaia marked this pull request as draft February 3, 2025 19:24
Copy link
Contributor

github-actions bot commented Feb 3, 2025

Coverage Report for ./apps/cli

Status Category Percentage Covered / Total
🔵 Lines 31% 293 / 945
🔵 Statements 31.13% 298 / 957
🔵 Functions 36.95% 51 / 138
🔵 Branches 27.88% 121 / 434
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/cli/src/config.ts 89.5% 82.02% 96.77% 89.3% 66-67, 215, 222, 231, 246-249, 260, 269, 271, 280, 318, 328-332, 344
Generated in workflow #264 for commit fda0e28 by the Vitest Coverage Report Action

@endersonmaia endersonmaia force-pushed the feature/add-espresso-reader-to-cli branch from 0a1e320 to 4bf2c98 Compare February 4, 2025 14:42
@miltonjonat
Copy link

Instructions fixing typos :)

pnpm build --filter @cartesi/cli
alias cartesi-dev=$PWD/apps/cli/bin/run.js
cartesi-dev create --branch=prerelease/sdk-12 --template=go my-go-dapp-v2 
cd my-go-dapp-v2
cartesi-dev build
cartesi-dev run --enable-espresso

Copy link

@miltonjonat miltonjonat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't access the espresso endpoint on http://localhost:8080/espresso, only directly on http://localhost:8770. Is that expected?

apps/cli/src/node/docker-compose-espresso.yaml Outdated Show resolved Hide resolved
@endersonmaia
Copy link
Contributor Author

I couldn't access the espresso endpoint on http://localhost:8080/espresso, only directly on http://localhost:8770. Is that expected?

It's missing some instructions. We expose 4 espresso API endpoints via names PATHs, they are:

  • http://localhost:8080/espresso/sequencer
  • http://localhost:8080/espresso/builder
  • http://localhost:8080/espresso/prover
  • http://localhost:8080/espresso/dev

So if you wanna call the /v0/status/block-height you should call

curl http://localhost:8080/espresso/sequencer/v0/status/block-height

DATABASE_URL: postgres://postgres:password@database:5432/espresso_reader
ESPRESSO_BASE_URL: http://espresso:8770
ESPRESSO_NAMESPACE: 51025
ESPRESSO_STARTING_BLOCK: 101

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rollups-espresso-reader does a weird thing: it publishes two endpoints /nonce and /submit on a port configurable with env ESPRESSO_SERVICE_ENDPOINT (default I think is 8080). We'd need to expose these too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about this format?

  • /espresso/reader/nonce
  • /espresso/reader/submit

If the developer interaction if gonna be focused on those endpoints, we could even hide everything from espresso-dev-node and only publish those at /espresso/submit and /espresso/nonce, also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miltonjonat I sent a fixup where you can access those endpoints at:

  • http://localhost:8080/espresso/reader/nonce
  • http://localhost:8080/espresso/reader/submit

Please, let me know if it works!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, these endpoints are currently conveniences for the Espresso integration, and strictly speaking they could (or should?) be part of another component separate from the reader itself. So, no, I wouldn't use "reader" there.

Frankly, I think it's also quite odd to put it under "espresso" in this context, because that "espresso" there is meant to be the (local) Espresso Network, while these are endpoints on the Node itself.

Tbh, the idea here was also that the Node would provide /nonce and /submit functionalities as something more universal (i.e., to also be used when integrated with Avail, Celestia, etc). And that clients wouldn't even know which alt-DA was being used by the app/node.

TL;DR, I'd use http://localhost:8080/nonce and http://localhost:8080/submit directly; or we could come up with a nice namespace for convenience endpoints of this kind (maybe http://localhost:8080/tx/nonce?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand there is no unified way to send a transaction. anvil expects ethereum transactions as specified by eth_sendTransaction, espresso expects a transaction object, avail has its own spec.

So having a single /nonce and /submit at the root of the service, regardless the configured DA, with various formats will be extremely confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm exposing those espresso-dev-node ports to the host to test if thins are working, but they should be removed on the final version of this PR togethers with the /espresso endpoints for the proxy.

About the /nonce and /submit endpoints from the espresso-reader, I need a definition so I can apply in this PR.

Copy link

@miltonjonat miltonjonat Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand there is no unified way to send a transaction. anvil expects ethereum transactions as specified by eth_sendTransaction, espresso expects a transaction object, avail has its own spec.

So having a single /nonce and /submit at the root of the service, regardless the configured DA, with various formats will be extremely confusing.

I see this differently. With Paio, we standardized an EIP-712 structure to be used when submitting tx's. This structure includes the target chainId and app address, nonce, and envisions payment info too. Avail and Celestia integrations would use this via Paio, and we adopted it for the Espresso integration as well (Espresso itself doesn't have any spec on the payload you submit, which doesn't even need to be signed atm). The idea is that you would use /nonce and /submit for all L2 tx submissions (i.e., that use Paio / alt-DA), while of course clients would still send direct L1 Eth tx's via the InputBox for L1->L2 messages.

Now, another interesting discussion would be to provide a standard Eth JSON-RPC API on the Node for these L2 tx's. I'd like that, but I don't know how viable it is. It would be part of that redesign discussion for all this architecture.

Copy link

@miltonjonat miltonjonat Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, one path what would be very adherent to the current architecture would be something like L2tx, leading to http://localhost:8080/L2tx/nonce and http://localhost:8080/L2tx/submit (I suggested tx above, but I admit that L2tx would be more precise)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you going to use a single format for DAs with different requirements?
I.e. how do you define the namespace in case of Espresso?

Copy link

@miltonjonat miltonjonat Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current design, the client/user does not inform any DA-specific parameters in the payload it submits. Whatever is required to submit to the alt-DA configured for the application should be given by DA-specific parameters defined upon deployment of the app, and stored as on-chain app configuration. The Node will fetch that from the application contract (it's currently defined as an ABI encoded method+parameters), and thus know how to handle tx submission when /submit is called.

@endersonmaia endersonmaia force-pushed the feature/add-espresso-reader-to-cli branch from 562e089 to 78886ad Compare February 6, 2025 14:21
@endersonmaia endersonmaia force-pushed the feature/add-espresso-reader-to-cli branch from dc4d99a to fda0e28 Compare February 7, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants